feat: pwa pd support#3654
feat: pwa pd support#3654mjuraschik wants to merge 3 commits intoSalesforceCommerceCloud:developfrom
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
8c5aeec to
0414239
Compare
aeebf74 to
f3f8dc8
Compare
| ))} | ||
| </SimpleGrid> | ||
| ) | ||
| export const MobileGrid1r1c = ({regions}) => { |
There was a problem hiding this comment.
I will delete the new layout components again, they are here for testing
4dc646c to
82f0659
Compare
| // @ts-ignore | ||
| isVisible: Boolean(component.visible), | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore |
There was a problem hiding this comment.
why using ignore ts here?
There was a problem hiding this comment.
Because this is a valid attribute, but I think pwa kit has the latest state of that yet, neither has the commerce-sdk. I should probably fix that, but I am not sure what the process is. Can you help me with that?
There was a problem hiding this comment.
I made it less ugly and not use a ts-ignore though
| } | ||
|
|
||
| return ( | ||
| <Suspense |
There was a problem hiding this comment.
What are the ways to tests if Suspense can render fallback component? Correctly if I am wrong, but pwa-kit is currently SSR and not support streaming at all. Using Suspense will probably not work for pwa-kit.
There was a problem hiding this comment.
oh, you're right. I changed it to only render in that suspense on the client. not sure whether this would be best practice here. I could also just remove it if you want
There was a problem hiding this comment.
You can simply return the it with a div wrapper
<div>
{componentElement}
</div>
Let's keep the displayName (which existing in other components, it is to make it easier for debugging)
Component.displayName = 'Component'
packages/commerce-sdk-react/src/components/ShopperExperience/registry.ts
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,78 @@ | |||
| /* | |||
There was a problem hiding this comment.
Why can't this component be part of commerce-sdk-react or/and be part of PageDesignerProvider?
I am thinking on the UX for dev when they need to integrate PD to the template, they have to set up two things, one is the PageDesignerProvider and PageDesignerInit to get it works? Is there way we can set up so they can simply do one step?
const App = () = {
return <PageDesignerSetup />
}
// commermce-react-sdk
const PageDesignerSetup = () => {
// everything needed to init PD for template. E.g
<PageDesignerProvider clientId="pwa-kit" targetOrigin="*" usid={usid}>
<PageDesignerInit />
<YourAppContent />
</PageDesignerProvider>
}
There was a problem hiding this comment.
The page designer init code has a dependency to react-router, which is something I didn't want to pull into the commerce-sdk
|
Can you please add some steps how I can test drive these code changes? |
steelsojka
left a comment
There was a problem hiding this comment.
I think this looks good from a PD standpoint.
82f0659 to
c61636a
Compare
|
Code deployed here https://cc-phoenix-dreamforce-demo.sfdc-ckzqgc-ecom1.exp-delivery-soak.com/ with some adjustments to the homepage that I did not commit. The homepage in the storefront deployed there, is connecting to page designer to get data. The components you see are supposed to match the sfra example components. Other pages should continue to work as before. |
2ffcbea to
a2b1793
Compare
a2b1793 to
70cfd91
Compare
70cfd91 to
7fcff78
Compare
Description
Page Designer OOTB Support with PWA:
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization